-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
perf: Add NSID column #10
base: master
Are you sure you want to change the base?
Conversation
2a78fa5
to
27a6677
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good changes, just few comments.
Also I noticed that example of output in your commit message doesn't have a closing ")" (the opening one is "RDMA (". Is it copy-paste mistake or this bracket is absent in perf output?
examples/nvme/perf/perf.c
Outdated
@@ -747,6 +762,7 @@ register_ns(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_ns *ns) | |||
} | |||
|
|||
build_nvme_name(entry->name, sizeof(entry->name), ctrlr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You already call build_nvme_name
in build_nvme_ns_name
, no need to keep it
examples/nvme/perf/perf.c
Outdated
|
||
res = build_nvme_name(name, length, ctrlr); | ||
if (res > 0) { | ||
snprintf(name + res - 1, length - res + 1, " NSID %d", nsid); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I know, snprintf returns the number of char written without termination symbol, so why do we need these +1
and -1
? I might be wrong, but it seems that we need just name + res, length - res
Also please correct format of nsid, it should be %u
examples/nvme/perf/perf.c
Outdated
@@ -1243,7 +1259,7 @@ print_performance(void) | |||
worker = worker->next; | |||
} | |||
|
|||
printf("========================================================\n"); | |||
printf("==========================================================================\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to add additional =
characters. Let's remove them and add if someone asks to add them during review in upstream.
Currently an option is implemented to monitor a correlation between the core and namespace. Previously: ================================================================== Device Information RDMA (addr:1.1.75.2 subnqn:nqn.2016-06.io.spdk:cnode1 from core 0: RDMA (addr:1.1.75.2 subnqn:nqn.2016-06.io.spdk:cnode1 from core 0: ================================================================== Now: ======================================================================== Device Information RDMA (addr:1.1.75.2 subnqn:nqn.2016-06.io.spdk:cnode1 NSID 1 from core 0: RDMA (addr:1.1.75.2 subnqn:nqn.2016-06.io.spdk:cnode1 NSID 2 from core 0: ======================================================================== Signed-off-by: Alla Kiseleva <[email protected]>
27a6677
to
7027a37
Compare
Not all JSON methods require 'params' field to be supplied. Verification of the JSON is done on server side in parse_single_request(). We should not attempt to process garbage values on correct JSON config file during app start. Segfault can be observed if following valid JSON config is supplied: { "method": "framework_wait_init" } Resulting in: json_config.c:388:13: runtime error: applying non-zero offset 18446744073709551600 to null pointer AddressSanitizer:DEADLYSIGNAL ================================================================= ==3386067==ERROR: AddressSanitizer: SEGV on unknown address (pc 0x0000007260ff bp 0x7ffe6ea06890 sp 0x7ffe6ea067e0 T0) ==3386067==The signal is caused by a READ memory access. ==3386067==Hint: this fault was caused by a dereference of a high value address (see register values below). Dissassemble the provided pc to learn which register was used. #0 0x7260ff in app_json_config_load_subsystem_config_entry /home/tzawadzk/spdk/lib/event/json_config.c:391 #1 0x7cbb13 in msg_queue_run_batch /home/tzawadzk/spdk/lib/thread/thread.c:505 #2 0x7cd00a in thread_poll /home/tzawadzk/spdk/lib/thread/thread.c:581 #3 0x7cfe18 in spdk_thread_poll /home/tzawadzk/spdk/lib/thread/thread.c:689 #4 0x71d6ef in _reactor_run /home/tzawadzk/spdk/lib/event/reactor.c:326 #5 0x71eb00 in reactor_run /home/tzawadzk/spdk/lib/event/reactor.c:382 #6 0x71f911 in spdk_reactors_start /home/tzawadzk/spdk/lib/event/reactor.c:477 #7 0x718237 in spdk_app_start /home/tzawadzk/spdk/lib/event/app.c:691 #8 0x407e94 in main /home/tzawadzk/spdk/app/spdk_tgt/spdk_tgt.c:120 #9 0x7f0f2eef2041 in __libc_start_main ../csu/libc-start.c:308 #10 0x4079ad in _start (/home/tzawadzk/spdk/build/bin/spdk_tgt+0x4079ad) Signed-off-by: Tomasz Zawadzki <[email protected]> Change-Id: I7ef1a764467817ad788fdf5dbe17eaeb99dcc22e Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3256 Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins <[email protected]> Reviewed-by: Jim Harris <[email protected]> Reviewed-by: Shuhei Matsumoto <[email protected]>
The controller data structure may be freed before subsystem resume done callback, we can take endpoint as the input parameter to avoid this issue. AddressSanitizer: heap-use-after-free on address 0x625000046100 at pc 0x00000082818f bp 0x7fff7b09bd10 sp 0x7fff7b09bd00 READ of size 8 at 0x625000046100 thread T0 (reactor_0) #0 0x82818e in vfio_user_dev_quiesce_resume_done /spdk/lib/nvmf/vfio_user.c:2147 #1 0x782cc0 in subsystem_state_change_done /spdk/lib/nvmf/subsystem.c:634 #2 0xad047b in _call_completion /spdk/lib/thread/thread.c:2344 #3 0xabc48d in msg_queue_run_batch /spdk/lib/thread/thread.c:710 #4 0xac0670 in thread_poll /spdk/lib/thread/thread.c:926 #5 0xac0ead in spdk_thread_poll /spdk/lib/thread/thread.c:986 #6 0x9a5b4f in _reactor_run /spdk/lib/event/reactor.c:920 #7 0x9a6442 in reactor_run /spdk/lib/event/reactor.c:958 #8 0x9a717c in spdk_reactors_start /spdk/lib/event/reactor.c:1060 #9 0x99884a in spdk_app_start /spdk/lib/event/app.c:643 #10 0x407e82 in main /spdk/app/nvmf_tgt/nvmf_main.c:75 #11 0x7f822095ff42 in __libc_start_main (/lib64/libc.so.6+0x23f42) #12 0x407abd in _start (/spdk/build/bin/nvmf_tgt+0x407abd) 0x625000046100 is located 0 bytes inside of 8320-byte region [0x625000046100,0x625000048180) freed by thread T0 (reactor_0) here: #0 0x7f82219ff91f in __interceptor_free (/lib64/libasan.so.5+0x10d91f) #1 0x837059 in _free_ctrlr /spdk/lib/nvmf/vfio_user.c:2976 #2 0x837327 in free_ctrlr /spdk/lib/nvmf/vfio_user.c:2996 #3 0x843541 in nvmf_vfio_user_close_qpair /spdk/lib/nvmf/vfio_user.c:3742 #4 0x7d1d91 in nvmf_transport_qpair_fini /spdk/lib/nvmf/transport.c:604 #5 0x7ad922 in _nvmf_qpair_destroy /spdk/lib/nvmf/nvmf.c:1055 #6 0x761362 in nvmf_qpair_request_cleanup /spdk/lib/nvmf/ctrlr.c:4026 #7 0x761906 in spdk_nvmf_request_free /spdk/lib/nvmf/ctrlr.c:4041 #8 0x75a931 in nvmf_qpair_free_aer /spdk/lib/nvmf/ctrlr.c:3576 #9 0x7ae626 in spdk_nvmf_qpair_disconnect /spdk/lib/nvmf/nvmf.c:1127 #10 0x83db36 in _vfio_user_qpair_disconnect /spdk/lib/nvmf/vfio_user.c:3433 #11 0xabc48d in msg_queue_run_batch /spdk/lib/thread/thread.c:710 #12 0xac0670 in thread_poll /spdk/lib/thread/thread.c:926 #13 0xac0ead in spdk_thread_poll /spdk/lib/thread/thread.c:986 #14 0x9a5b4f in _reactor_run /spdk/lib/event/reactor.c:920 #15 0x9a6442 in reactor_run /spdk/lib/event/reactor.c:958 #16 0x9a717c in spdk_reactors_start /spdk/lib/event/reactor.c:1060 #17 0x99884a in spdk_app_start /spdk/lib/event/app.c:643 #18 0x407e82 in main /spdk/app/nvmf_tgt/nvmf_main.c:75 #19 0x7f822095ff42 in __libc_start_main (/lib64/libc.so.6+0x23f42) previously allocated by thread T0 (reactor_0) here: #0 0x7f82219fff16 in __interceptor_calloc (/lib64/libasan.so.5+0x10df16) #1 0x837413 in nvmf_vfio_user_create_ctrlr /spdk/lib/nvmf/vfio_user.c:3010 #2 0x83bc68 in nvmf_vfio_user_accept /spdk/lib/nvmf/vfio_user.c:3313 #3 0xabfbd8 in thread_execute_timed_poller /spdk/lib/thread/thread.c:872 #4 0xac0c75 in thread_poll /spdk/lib/thread/thread.c:960 #5 0xac0ead in spdk_thread_poll /spdk/lib/thread/thread.c:986 #6 0x9a5b4f in _reactor_run /spdk/lib/event/reactor.c:920 #7 0x9a6442 in reactor_run /spdk/lib/event/reactor.c:958 #8 0x9a717c in spdk_reactors_start /spdk/lib/event/reactor.c:1060 #9 0x99884a in spdk_app_start /spdk/lib/event/app.c:643 #10 0x407e82 in main /spdk/app/nvmf_tgt/nvmf_main.c:75 #11 0x7f822095ff42 in __libc_start_main (/lib64/libc.so.6+0x23f42) SUMMARY: AddressSanitizer: heap-use-after-free /spdk/lib/nvmf/vfio_user.c:2147 in vfio_user_dev_quiesce_resume_done Change-Id: Icf5e5b360b9107a3c5eb960ae59b7fe10ace1c66 Signed-off-by: Changpeng Liu <[email protected]> Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/11420 Community-CI: Broadcom CI <[email protected]> Tested-by: SPDK CI Jenkins <[email protected]> Reviewed-by: Dong Yi <[email protected]> Reviewed-by: John Levon <[email protected]> Reviewed-by: Ben Walker <[email protected]> Reviewed-by: Jim Harris <[email protected]>
c3f17a8
to
485f9f9
Compare
Ubsan with clang complains when using spdk_ioviter with more iters than declared in the array: iov.c:69:9: runtime error: index 3 out of bounds for type 'struct spdk_single_ioviter[2]' #0 0x5df709 in spdk_ioviter_firstv /home/vagrant/spdk_repo/spdk/lib/util/iov.c:69:9 #1 0x53780b in raid5f_xor_stripe /home/vagrant/spdk_repo/spdk/module/bdev/raid/raid5f.c:270:24 #2 0x531bd8 in raid5f_submit_write_request /home/vagrant/spdk_repo/spdk/module/bdev/raid/raid5f.c:520:2 #3 0x52a03a in raid5f_submit_rw_request /home/vagrant/spdk_repo/spdk/module/bdev/raid/raid5f.c:596:9 #4 0x548c17 in test_raid5f_write_request /home/vagrant/spdk_repo/spdk/test/unit/lib/bdev/raid/raid5f.c/raid5f_ut.c:550:2 #5 0x544e18 in test_raid5f_submit_rw_request /home/vagrant/spdk_repo/spdk/test/unit/lib/bdev/raid/raid5f.c/raid5f_ut.c:714:3 #6 0x553e61 in __test_raid5f_submit_full_stripe_write_request /home/vagrant/spdk_repo/spdk/test/unit/lib/bdev/raid/raid5f.c/raid5f_ut.c:878:3 #7 0x543f84 in run_for_each_raid5f_config /home/vagrant/spdk_repo/spdk/test/unit/lib/bdev/raid/raid5f.c/raid5f_ut.c:748:3 #8 0x527ac1 in test_raid5f_submit_full_stripe_write_request /home/vagrant/spdk_repo/spdk/test/unit/lib/bdev/raid/raid5f.c/raid5f_ut.c:885:2 #9 0x7f4a71a0960a (/usr/lib64/libcunit.so.1+0x460a) (BuildId: 9c82dd336cbccd99721651ac0a04435e746e0fc0) #10 0x7f4a71a09937 (/usr/lib64/libcunit.so.1+0x4937) (BuildId: 9c82dd336cbccd99721651ac0a04435e746e0fc0) #11 0x7f4a71a0a897 in CU_run_all_tests (/usr/lib64/libcunit.so.1+0x5897) (BuildId: 9c82dd336cbccd99721651ac0a04435e746e0fc0) #12 0x524fe8 in main /home/vagrant/spdk_repo/spdk/test/unit/lib/bdev/raid/raid5f.c/raid5f_ut.c:1006:2 #13 0x7f4a711d750f in __libc_start_call_main (/usr/lib64/libc.so.6+0x2750f) (BuildId: 81daba31ee66dbd63efdc4252a872949d874d136) #14 0x7f4a711d75c8 in __libc_start_main@GLIBC_2.2.5 (/usr/lib64/libc.so.6+0x275c8) (BuildId: 81daba31ee66dbd63efdc4252a872949d874d136) #15 0x4235b4 in _start (/home/vagrant/spdk_repo/spdk/test/unit/lib/bdev/raid/raid5f.c/raid5f_ut+0x4235b4) (BuildId: 028d075edd1a7cd17881fd678ef076adfdbac13d) Fix this by making iters a zero-length array and put it in a union with a two-element array to keep the default size for compatibility. Change-Id: I8573b015755e9986cdadbfa1705d269d51a7c2b7 Signed-off-by: Artur Paszkiewicz <[email protected]> Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/18402 Reviewed-by: Jim Harris <[email protected]> Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins <[email protected]> Reviewed-by: Shuhei Matsumoto <[email protected]>
As per typedef in nvme.h the spdk_nvme_cpl argument should be a pointer to a const struct. This fixes runtimer error under clang >= 17.x which now makes the -fsanitize=function available for C and which on our end is being enabled via -fsanitize=undefined under UBSAN. Error in question: Test: test_spdk_nvme_detach ...passed Test: test_nvme_completion_poll_cb ...passed Test: test_nvme_user_copy_cmd_complete .../root/spdk/lib/nvme/nvme.c:417:2: runtime error: call to function dummy_cb through pointer to incorrect function type 'void (*)(void *, const struct spdk_nvme_cpl *)' /root/spdk/test/unit/lib/nvme/nvme.c/nvme_ut.c:584: note: dummy_cb defined here #0 0x5098e0 in nvme_user_copy_cmd_complete /root/spdk/lib/nvme/nvme.c:417:2 #1 0x532161 in test_nvme_user_copy_cmd_complete /root/spdk/test/unit/lib/nvme/nvme.c/nvme_ut.c:604:2 #2 0x7f08c952266a (/usr/lib64/libcunit.so.1+0x466a) (BuildId: d99e3b60795f2ce01ada820b4b7e3cd84d8150fe) #3 0x7f08c95229c7 (/usr/lib64/libcunit.so.1+0x49c7) (BuildId: d99e3b60795f2ce01ada820b4b7e3cd84d8150fe) #4 0x7f08c9523a9f in CU_run_all_tests (/usr/lib64/libcunit.so.1+0x5a9f) (BuildId: d99e3b60795f2ce01ada820b4b7e3cd84d8150fe) #5 0x55555e in run_tests /root/spdk/lib/ut/ut.c:169:3 #6 0x552aec in spdk_ut_run_tests /root/spdk/lib/ut/ut.c:225:8 #7 0x522d52 in main /root/spdk/test/unit/lib/nvme/nvme.c/nvme_ut.c:1664:17 #8 0x7f08c8c28149 in __libc_start_call_main (/usr/lib64/libc.so.6+0x28149) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756) #9 0x7f08c8c2820a in __libc_start_main@GLIBC_2.2.5 (/usr/lib64/libc.so.6+0x2820a) (BuildId: 7ea8d85df0e89b90c63ac7ed2b3578b2e7728756) #10 0x42b6a4 in _start (/root/spdk/test/unit/lib/nvme/nvme.c/nvme_ut+0x42b6a4) (BuildId: 6fc2caaf777030becad2d0f660ec68443f3380b4) SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /root/spdk/lib/nvme/nvme.c:417:2 in ./test/unit/unittest.sh: line 85: 75349 Aborted (core dumped) $valgrind $testdir/lib/nvme/nvme.c/nvme_ut Change-Id: Iddbd5fc0dee0ef6a6cc1f032e079f6119e76aed9 Signed-off-by: Michal Berger <[email protected]> Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/22025 Reviewed-by: Jim Harris <[email protected]> Community-CI: Mellanox Build Bot Reviewed-by: Konrad Sztyber <[email protected]> Tested-by: SPDK CI Jenkins <[email protected]>
Can one of the admins verify this patch? |
There was no correlation between core and namespace before
Signed-off-by: Alla Kiseleva [email protected]